Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add "from" field to rule schema PEOBS-149 #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

somic
Copy link

@somic somic commented Nov 6, 2018

Details and example json are in internal PEOBS-149

Copy link

@thebostik thebostik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to add more code so that the From rule is preserved and round-tripped correctly?

Based on naming, I have a feeling it will probably be very similar to how we treat To across the provider
https://github.com/Yelp/terraform-provider-cloudhealth/blob/master/cloudhealth/json_to_tf.go#L82

@somic
Copy link
Author

somic commented Nov 6, 2018

I am not entirely confident what this field stands for and whether it holds any significance, all I know is it can trip parsing.

I was hoping to follow YAGNI and address the issue at hand, without trying to round-trip it since I don't think it's significant.

@thebostik
Copy link

It's not blocking us from working on the important perspectives so there is no urgent rush for a partial fix. Understanding what this field does is an important step IMO before we can decide we won't need it.

Without support, and it getting added on its own mysteriously, implies we're ok to keep re-applying changes to this resource whenever it reappears, which has been at least twice now IIRC.

When I test the provider built from this branch locally, I still get an error, did you see the same thing? I am not sure this is enough of a fix.
* cloudhealth_perspective.owner_tag: cloudhealth_perspective.owner_tag: Group reference 3092376474108 not found

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants